feat(cli): support session forking and history loading in CLI#20
Conversation
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a session-forking CLI path, introduces a new ChangesSession Fork Flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@microagents-cli/src/main.rs`:
- Around line 140-156: The fork flow in main currently clones parent events and
returns the new session id, but it never persists the SessionFork marker event.
After the child session is created in the event replay loop, add a write for the
SessionFork event through storage so the lineage is recorded distinctly from a
normal resumed session. Use the existing session-writing path in main alongside
the SessionInit and other event handling to place this marker at the right point
in the fork process.
- Around line 163-169: The forked startup logic in main.rs is treating
`--session-id` as the only way to continue into the TUI after `fork_session`,
which makes plain `--fork <parent>` exit early unless an unrelated value is
passed. Update the `resolved_session_id` handling so the forked TUI path uses
`Some(new_sid)` when `args.fork` is present and no prompt is provided, and
reserve the early return only for an explicit clone-only mode if you add one.
Keep the fix centered on `fork_session`, `resolved_session_id`, and the
`args.prompt` / `args.session_id` branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d1dc15b-0b86-4d31-acde-a6eaaebc6a2f
📒 Files selected for processing (1)
microagents-cli/src/main.rs
34c9ab8 to
09bd1ef
Compare
AstraBert
left a comment
There was a problem hiding this comment.
This looks good, thanks for simplifying the codebase and the to/from json-rpc conversion! gonna merge only this one since the other one contains basically the same logic for events to/from json-rpc conversion
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
microagents-events/src/lib.rs (1)
293-293: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the non-null
tool_callsserialization path.Line 293 now routes
AssistantResponseEventthrough generic serde serialization, but the existing serialization test only coverstool_calls: None. Add aSome(vec![...])serialization or round-trip assertion so the wire shape stays locked down after replacing the prior special handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@microagents-events/src/lib.rs` at line 293, The AssistantResponseEvent serialization change only has coverage for the tool_calls: None case, so add a test that exercises the non-null tool_calls path and verifies the JSON-RPC wire shape. Update the serialization/round-trip test around AssistantResponseEvent and serialize_to_jsonrpc to include tool_calls as Some(vec![...]) so the generic serde path stays locked down.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@microagents-cli/src/main.rs`:
- Around line 139-152: The history cloning in main.rs is copying inherited
SessionFork markers from the parent and then adding a new fork marker for the
child, which leaves the new session with two fork origins. Update the
parent_events replay loop so SessionFork events are skipped (or otherwise not
rewritten/persisted) while cloning history, and only append the new
child-to-parent fork marker once after the loop. Use the existing event handling
around cloned_event, SessionInit, and storage.update_session/create_session to
locate the change.
- Around line 49-51: The CLI options are ambiguous because `fork` currently
overrides `session_id` silently; update the argument definitions in `main.rs` so
`--fork` and `--session-id` cannot be used together. Add the conflict on the
`fork` field in the argument struct, using the existing `fork` and `session_id`
symbols, so the parser rejects the combination instead of choosing one
implicitly.
---
Nitpick comments:
In `@microagents-events/src/lib.rs`:
- Line 293: The AssistantResponseEvent serialization change only has coverage
for the tool_calls: None case, so add a test that exercises the non-null
tool_calls path and verifies the JSON-RPC wire shape. Update the
serialization/round-trip test around AssistantResponseEvent and
serialize_to_jsonrpc to include tool_calls as Some(vec![...]) so the generic
serde path stays locked down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72b1fbce-7f34-455d-8946-dd9fbfea3745
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
microagents-cli/Cargo.tomlmicroagents-cli/src/main.rsmicroagents-core/Cargo.tomlmicroagents-events/Cargo.tomlmicroagents-events/src/lib.rsmicroagents-storage/Cargo.tomlsearch-evals/Cargo.toml
✅ Files skipped from review due to trivial changes (1)
- search-evals/Cargo.toml
Adds a
--fork <session_id>CLI flag to allow initializing a new session from an existing parent session's history.This copies events from the specified parent session to the new child session on startup and records a
SessionForkevent.Key Changes:
--forkoption to CLI arguments.Summary by CodeRabbit
New Features
--fork <ID>option to create a new session seeded with the selected session’s saved history.session.forkevent as part of the updated event protocol.Bug Fixes